Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix leak in Vec::extend_from_within #82760

Merged
merged 2 commits into from
Mar 13, 2021

Conversation

WaffleLapkin
Copy link
Member

Fixes #82533

Previously vec's len was updated only after full copy, making the method
leak if T::clone panic!s.

This commit makes `Vec::extend_from_within` (or, more accurately, it's
`T: Clone` specialization) update vec's len on every iteration, fixing
the issue.

`T: Copy` specialization was not affected by the issue b/c it doesn't
call user specified code (as, e.g. `T::clone`), and instead calls
`ptr::copy_nonoverlapping`.
@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2021
@kennytm
Copy link
Member

kennytm commented Mar 13, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 13, 2021

📌 Commit 1f031d9 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2021
@bors
Copy link
Contributor

bors commented Mar 13, 2021

⌛ Testing commit 1f031d9 with merge ec487bf...

@bors
Copy link
Contributor

bors commented Mar 13, 2021

☀️ Test successful - checks-actions
Approved by: kennytm
Pushing ec487bf to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 13, 2021
@bors bors merged commit ec487bf into rust-lang:master Mar 13, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 13, 2021

/// Safety: changing returned .2 (&mut usize) is considered the same as calling `.set_len(_)`.
///
/// This method is used to have unique access to all vec parts at once in `extend_from_within`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is used to have/provides/ would be clearer

@WaffleLapkin WaffleLapkin deleted the unleak_extend_from_within branch March 24, 2021 09:21
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2021
…vink

Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by `@pickfire` in rust-lang#82760
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2021
…vink

Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by ``@pickfire`` in rust-lang#82760
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 18, 2021
…vink

Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by ```@pickfire``` in rust-lang#82760
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2021
Slightly change wording in doc comment and fix typo in vec/mod.rs

Suggested by `@pickfire` in rust-lang#82760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extend_from_within leaks elements on panic
6 participants